rustc_expand: improve diagnostics for non-repeatable metavars#154014
rustc_expand: improve diagnostics for non-repeatable metavars#154014rust-bors[bot] merged 1 commit intorust-lang:mainfrom
Conversation
|
r? @adwinwhite rustbot has assigned @adwinwhite. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
r? @estebank |
This comment has been minimized.
This comment has been minimized.
a2e1edc to
1bfa30d
Compare
This comment has been minimized.
This comment has been minimized.
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rustc_expand: improve diagnostics for non-repeatable metavars
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (c7819c5): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResults (secondary -0.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.1%, secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 482.731s -> 482.467s (-0.05%) |
|
The regressions here are all in html5ever. I think this should be fine. @Unique-Usman could you provide a description, a link to the previous PR and squash the commits? |
|
@estebank, I will do that. Thanks. |
Co-authored-by: Esteban Küber <esteban@kuber.com.ar> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
35d11f3 to
ed4a1f8
Compare
|
@bors try @rust-timer queue getting some confirmation on the html5ever incr regression |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rustc_expand: improve diagnostics for non-repeatable metavars
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (9837802): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary -2.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -0.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.1%, secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 484.31s -> 483.312s (-0.21%) |
|
It consistently affects html5ever incr builds a little bit (we are doing more after all), but I think the output improvement is worth it. @bors r+ |
This comment has been minimized.
This comment has been minimized.
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 80d0e4b (parent) -> 1174f78 (this PR) Test differencesShow 8 test diffsStage 1
Stage 2
Additionally, 3 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 1174f784096deb8e4ba93f7e4b5ccb7bb4ba2c55 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (1174f78): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowOur benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResults (secondary -0.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 481.885s -> 483.566s (0.35%) |
View all comments
There was an initally opened pr which solve this issue here #152679. It got merged but, there was a perf regression. And this new pr is opened to address the problem. The first did the computation of binding and matched_rule and then passed them as owned value down to
diagnostics::emit_frag_parse_err(but, now this pr address the issue by passinglhsandrulesas borrowed value to from_tts and the move the logic todiagnostics::emit_frag_parse_err(.Fix #47452.